Closed
Bug 1405141
Opened 8 years ago
Closed 8 years ago
Extend test verification to web-platform-tests
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: gbrown, Assigned: gbrown)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
jgraham points out that wpt is an important omission for test-verify.
I had originally thought that wpt was not high value because those tests are often modified outside of mozilla integration, then verified and integrated en-masse, but web-platform tests are, of course, also updated at Mozilla and pushed to hg, just like other test suites. We should try to support them like other test types.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•8 years ago
|
||
Pushed a couple of lightly-tested commits for the internals of this to mozreview. Please consider this more like f? than r?; I don't expect the patches to land as-is, but I thought it might speed things up here if I did the first cut at the harness-internal parts. Do you have time to look at the try integration and check that my implementation more or less matches the other harnesses?
Flags: needinfo?(gbrown)
![]() |
Assignee | |
Comment 4•8 years ago
|
||
Thanks much James, this is a great help. I'm happy to implement the taskcluster/mozharness parts and do integration testing and I should be able to look at that soon.
I'll make a comment or two on your patches tonight, then have a closer look tomorrow.
Flags: needinfo?(gbrown)
![]() |
Assignee | |
Comment 5•8 years ago
|
||
mozreview-review |
Comment on attachment 8918847 [details]
Bug 1405141 - Add support for rerunning web-platform-tests without restarting,
https://reviewboard.mozilla.org/r/189714/#review195142
::: testing/web-platform/tests/tools/wptrunner/wptrunner/wptcommandline.py:99
(Diff revision 1)
> help="run under a debugger, e.g. gdb or valgrind")
> debugging_group.add_argument('--debugger-args', help="arguments to the debugger")
> + debugging_group.add_argument("--rerun", action="store", type=int, default=1,
> + help="Number of times to re run each test without restarts")
> debugging_group.add_argument("--repeat", action="store", type=int, default=1,
> - help="Number of times to run the tests")
> + help="Number of times to run the tests, restarting between each run")
For mochitest and reftest, --repeat means re-run the test without restarts (in one browser session), so I'm a little concerned that this could be confusing.
Does repeat-until-unexpected restart the browser between runs?
![]() |
Assignee | |
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8918848 [details]
Bug 1405141 - Add stability checking to wptrunner,
https://reviewboard.mozilla.org/r/189716/#review195146
::: testing/web-platform/tests/tools/wptrunner/wptrunner/stability.py:209
(Diff revision 1)
> + with open("raw.log", "rb") as log:
> + results, inconsistent = process_results(log, iterations)
> + return results, inconsistent, iterations
> +
> +
> +def get_steps(logger, repeat_loop, repeat_restart, kwargs_extras):
I like "repeat_loop" / "repeat_restart" -- nice and explicit.
::: testing/web-platform/tests/tools/wptrunner/wptrunner/stability.py:246
(Diff revision 1)
> + log('::: Test verification %s' % final_result)
> +
> + logger.info(':::')
> +
> +
> +def check_stability(logger, repeat_loop=10, repeat_restart=5, chaos_mode=True, max_time=None,
Those 10 and 5 counts are consistent with mochitest and reftest --verify support and probably a reasonable choice. For what it's worth, I don't think it is necessary to keep steps or repeat counts consistent across test suites / harnesses. It is fine if they are, and that makes things easy to remember, but don't feel constrained to those defaults.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
![]() |
Assignee | |
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8918847 [details]
Bug 1405141 - Add support for rerunning web-platform-tests without restarting,
https://reviewboard.mozilla.org/r/189714/#review196938
Testing locally, it seems --rerun doesn't always work. If I run
mach wpt netinfo/netinfo-basics.html --rerun 5
the test seems to run once and then pause, with console output:
Thread-TestrunnerManager-1 INFO Pausing until the browser exits
If I manually close the browser, I get:
0:20.13 LOG: Thread-TestrunnerManager-1 ERROR Traceback (most recent call last):
File "/home/gbrown/src/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 352, in run
new_state = self.wait_event()
File "/home/gbrown/src/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 426, in wait_event
return f(*data)
File "/home/gbrown/src/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py", line 596, in wait_finished
return self.after_test_end(True)
TypeError: after_test_end() takes exactly 3 arguments (2 given)
0:20.16 LOG: Thread-TestrunnerManager-1 INFO Browser exited with return code -15
Attachment #8918847 -
Flags: review?(gbrown)
![]() |
Assignee | |
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8918848 [details]
Bug 1405141 - Add stability checking to wptrunner,
https://reviewboard.mozilla.org/r/189716/#review196940
This works great for me. You noted "More work is needed to avoid duplicating all the code" -- did you want to tackle that now?
::: testing/web-platform/tests/tools/wptrunner/wptrunner/stability.py:178
(Diff revision 2)
> + if restart_after_iteration:
> + kwargs["repeat"] = iterations
> + else:
> + kwargs["rerun"] = iterations
> +
> + kwargs["pause_after_test"] = False
Maybe pause_after_test = False should be part of --rerun?
Attachment #8918848 -
Flags: review?(gbrown) → review+
![]() |
Assignee | |
Comment 11•8 years ago
|
||
BTW, I am making (slow) progress on integration with the taskcluster job, particularly determining if a modified file is a web-platform test.
![]() |
Assignee | |
Comment 12•8 years ago
|
||
I just realized that wpt has its own mozharness script, rather than running from desktop_unittest. That probably means we need a separate task: Something like TV (verifies mochitest, reftest, xpcshell) and TV-wpt (verifies wpt).
Comment 13•8 years ago
|
||
Any modification under testing/web-platform/tests or testing/web-platform/mozilla/tests is a wpt test modification. Upstream has a script that examines git history to determine which files were modified and which tests are affected by the modification. The affected tests part in particular might be good to reuse here; we could modify it to take a list of changes that we get from mercurial rather than git, perhaps. I don't know how much work that is, but I could look at it.
In terms of jobs, making this separate from other TV jobs probably does make sense from an implementation point of view. I sort of assumed that there was one of these jobs per testsuite anyway.
The code duplication is mostly with other upstream tools and I think it makes sense to fix that in an upstream PR since it probably wants non-Mozilla reviewers. So I'll do that there.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•8 years ago
|
||
mozreview-review-reply |
Comment on attachment 8918847 [details]
Bug 1405141 - Add support for rerunning web-platform-tests without restarting,
https://reviewboard.mozilla.org/r/189714/#review195142
> For mochitest and reftest, --repeat means re-run the test without restarts (in one browser session), so I'm a little concerned that this could be confusing.
>
> Does repeat-until-unexpected restart the browser between runs?
I am a little loathe to change the established meaning in wpt at this time, since upstream and third-party tools depend on it. I might revisit that later if this is a problem and I can fix all the dependencies.
![]() |
Assignee | |
Comment 19•8 years ago
|
||
mozreview-review |
Comment on attachment 8918847 [details]
Bug 1405141 - Add support for rerunning web-platform-tests without restarting,
https://reviewboard.mozilla.org/r/189714/#review197390
Attachment #8918847 -
Flags: review?(gbrown) → review+
Comment 20•8 years ago
|
||
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/3b0ece162b19
Add support for rerunning web-platform-tests without restarting, r=gbrown
https://hg.mozilla.org/integration/autoland/rev/97a964bf7f99
Add stability checking to wptrunner, r=gbrown
![]() |
||
Comment 21•8 years ago
|
||
Backed out for wpt10 on linux stylo disabled debug exceedind the maximum run time:
https://hg.mozilla.org/integration/autoland/rev/e4a7c6646eb4fb4f74a8c1295de35193646ca6fe
https://hg.mozilla.org/integration/autoland/rev/90a44d2859c76b5d6a152e81c330acda05d48e9d
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=97a964bf7f990416e039fc9747e19e46f7a6dcea&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=139125613&repo=autoland
Flags: needinfo?(james)
Comment 22•8 years ago
|
||
Flags: needinfo?(james)
Comment 23•8 years ago
|
||
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0faad69400fa
Add support for rerunning web-platform-tests without restarting, r=gbrown
https://hg.mozilla.org/integration/mozilla-inbound/rev/00bec72d17c4
Add stability checking to wptrunner, r=gbrown
![]() |
||
Comment 24•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0faad69400fa
https://hg.mozilla.org/mozilla-central/rev/00bec72d17c4
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•